refactor(cli): migrate commands to RunE with a single exit funnel by Caesarsage · Pull Request #490 · microcks/microcks-cli

Caesarsage · 2026-07-09T14:13:02Z

depend on #489

Part of the Microcks CLI v2 work (#255). Stacked on PR #489.

All ten commands move from Run + scattered os.Exit/log.Fatal to RunE returning errors.
A single funnel — cmd.Handle, called only by main — prints to stderr and maps Failure Kind → exit code (cmd/exit.go). Nothing else exits.
A non-conforming contract test travels as the silent ErrTestFailed sentinel → exit 1 with no spurious error line (mirrors kubectl diff).

No command/flag interface changes; failure output and exit codes intentionally change.

Exit codes (new, user-visible)

0 = success / non-zero = failure still holds, and 1 still means "contract test failed". Operational failures now carry distinct codes so CI can branch on why:

Code	Meaning
0	success / contract conformed
1	contract test failed (a clean run)
2	usage — bad arguments or flags
11	connection — couldn't reach Microcks/Keycloak
12	API — server rejected the request / unusable response
13	not-found — remote resource missing
14	environment — local precondition (container runtime, image, readiness)
20	generic — unclassified failure

These codes were previously undocumented and inconsistent (1/2/20 interchangeably), so there is no prior contract to break. Documented in README.md + documentation/error-handling.md.

Signed-off-by: caesarsage <destinyerhabor6@gmail.com>

Caesarsage · 2026-07-09T14:17:11Z

HOLD

till PR #489 is review and merged

Vaishnav88sk · 2026-07-22T09:28:27Z

Stacked PR of #489

Vaishnav88sk

Please resolve the conflicts.

Also add copyright block for pkg/errors/error_test.go

Signed-off-by: caesarsage <destinyerhabor6@gmail.com>

Vaishnav88sk · 2026-07-29T14:48:20Z

Some need to check:

usage/help preservation is not applied consistently yet; some commands still return plain KindUsage errors instead of using the new usageErrorf(...) path
the PR description still says “No impact on the binary”, which is too strong given the user-visible exit-code/error-handling changes

Please clean up those last consistency issues and then I’m happy to re-check.

Caesarsage · 2026-07-29T15:28:44Z

Some need to check:

usage/help preservation is not applied consistently yet; some commands still return plain KindUsage errors instead of using the new usageErrorf(...) path

the PR description still says “No impact on the binary”, which is too strong given the user-visible exit-code/error-handling changes

Please clean up those last consistency issues and then I’m happy to re-check.

Thanks for the review. The usage/help preservation was only added on paths that already printed usage before this PR. So I wouldn’t call the remaining commands a consistency regression in this PR, since they did not previously have that behavior.
I’m happy to raise a follow-up PR to standardize usage/help output across all commands. For this PR, my intent was narrower: migrate commands from Run to RunE and route returned errors through the new centralized handler.

I agree the PR description should not say “No impact on the binary” so strongly, since the exit-code/error-handling behavior is user-visible. I’ll reword that part.

Vaishnav88sk · 2026-07-29T15:45:51Z

Thanks for the review. The usage/help preservation was only added on paths that already printed usage before this PR. So I wouldn’t call the remaining commands a consistency regression in this PR, since they did not previously have that behavior. I’m happy to raise a follow-up PR to standardize usage/help output across all commands. For this PR, my intent was narrower: migrate commands from Run to RunE and route returned errors through the new centralized handler.

I agree the PR description should not say “No impact on the binary” so strongly, since the exit-code/error-handling behavior is user-visible. I’ll reword that part.

Thanks, that clarification is fair.

I’m okay not blocking this PR on full usage/help standardization as the intent here is only to preserve prior behavior where it already existed. We can do the broader consistency cleanup in a follow-up PR.

But still:

reword the PR description to remove the “no impact on the binary” claim
fix the remaining unclassified error paths so they don’t fall through to generic exit code 20

Caesarsage · 2026-07-29T15:51:13Z

Thanks for the review. The usage/help preservation was only added on paths that already printed usage before this PR. So I wouldn’t call the remaining commands a consistency regression in this PR, since they did not previously have that behavior. I’m happy to raise a follow-up PR to standardize usage/help output across all commands. For this PR, my intent was narrower: migrate commands from Run to RunE and route returned errors through the new centralized handler.
I agree the PR description should not say “No impact on the binary” so strongly, since the exit-code/error-handling behavior is user-visible. I’ll reword that part.

Thanks, that clarification is fair.

I’m okay not blocking this PR on full usage/help standardization as the intent here is only to preserve prior behavior where it already existed. We can do the broader consistency cleanup in a follow-up PR.

But still:

reword the PR description to remove the “no impact on the binary” claim

fix the remaining unclassified error paths so they don’t fall through to generic exit code 20

done. Thank you

Caesarsage · 2026-07-29T15:52:43Z

I checked the remaining paths and the real issue is not the KindUsage returns inside RunE; those already map to exit code 2.

The remaining unclassified paths are Cobra-generated errors that happen before RunE runs, so they fall through to generic 20:

cmd/test.go: cobra.ExactArgs(3) -> microcks test
cmd/import.go: cobra.MaximumNArgs(1) -> microcks import a b
root/subcommand flag parsing -> unknown flags like microcks --bad
cmd/cmd.go: MarkFlagsRequiredTogether(...) -> one Keycloak flag without the other

Vaishnav88sk · 2026-07-30T07:12:14Z

I checked the remaining paths and the real issue is not the KindUsage returns inside RunE; those already map to exit code 2.

The remaining unclassified paths are Cobra-generated errors that happen before RunE runs, so they fall through to generic 20:

cmd/test.go: cobra.ExactArgs(3) -> microcks test

cmd/import.go: cobra.MaximumNArgs(1) -> microcks import a b

root/subcommand flag parsing -> unknown flags like microcks --bad

cmd/cmd.go: MarkFlagsRequiredTogether(...) -> one Keycloak flag without the other

Thanks!

So could you fix this in PR or separate?
Or should we keep it?

Caesarsage · 2026-07-30T08:02:25Z

I checked the remaining paths and the real issue is not the KindUsage returns inside RunE; those already map to exit code 2.
The remaining unclassified paths are Cobra-generated errors that happen before RunE runs, so they fall through to generic 20:

cmd/test.go: cobra.ExactArgs(3) -> microcks test

cmd/import.go: cobra.MaximumNArgs(1) -> microcks import a b

root/subcommand flag parsing -> unknown flags like microcks --bad

cmd/cmd.go: MarkFlagsRequiredTogether(...) -> one Keycloak flag without the other

Thanks!

So could you fix this in PR or separate? Or should we keep it?

a separate PR would done

Vaishnav88sk · 2026-07-30T08:39:37Z

a separate PR would done

Okay then, please do it so it will be clean code.
Ready to merge @Harsh4902

Harsh4902 · 2026-07-30T16:28:52Z

@Caesarsage Have you already raised a seperate PR for changes suggested by @Vaishnav88sk or you will work on that after merging?

Caesarsage · 2026-07-30T18:38:03Z

@Caesarsage Have you already raised a seperate PR for changes suggested by @Vaishnav88sk or you will work on that after merging?

After merging. With a couple of other PRs needed to used the CLI as vscode extension interface. Since the suggestions are nor blocker. Merging this now is a good idea

Suggestions are marked but they are not the blocker for merging.

Harsh4902

LGTM

Caesarsage added 4 commits July 9, 2026 14:55

feat(errors): add Failure Kind classification

3829a79

Signed-off-by: caesarsage <destinyerhabor6@gmail.com>

fix(connectors): return classified errors instead of panicking

179ecbc

Signed-off-by: caesarsage <destinyerhabor6@gmail.com>

refactor(cmd): route commands through RunE and a single exit funnel

9b38b1f

Signed-off-by: caesarsage <destinyerhabor6@gmail.com>

docs: document exit codes and error-handling model

4675819

Signed-off-by: caesarsage <destinyerhabor6@gmail.com>

Caesarsage requested review from Harsh4902, lbroudoux and yada as code owners July 9, 2026 14:13

Caesarsage temporarily deployed to Build July 9, 2026 14:13 — with GitHub Actions Inactive

Caesarsage mentioned this pull request Jul 9, 2026

refactor(cli): remove the legacy exit shim and enforce it in CI #491

Open

This was referenced Jul 22, 2026

feat(test): add --output (text/json/yaml/github-actions) for machine-readable results #488

Open

fix(cli): classify failures and partial de-panic in the client library #489

Merged

Vaishnav88sk suggested changes Jul 27, 2026

View reviewed changes

Comment thread cmd/testDryRun.go

Comment thread cmd/import.go

Comment thread cmd/login.go

Comment thread cmd/importDir.go

Comment thread pkg/connectors/keycloak_client.go

Comment thread pkg/connectors/microcks_client.go

Address review feedback

830a40a

Signed-off-by: caesarsage <destinyerhabor6@gmail.com>

Caesarsage temporarily deployed to Build July 27, 2026 10:40 — with GitHub Actions Inactive

Vaishnav88sk previously requested changes Jul 29, 2026

View reviewed changes

Comment thread cmd/testExecutor.go

Harsh4902 approved these changes Jul 31, 2026

View reviewed changes

Harsh4902 merged commit a115dee into microcks:master Jul 31, 2026
5 checks passed

Uh oh!

Uh oh!

Conversation

Caesarsage commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Exit codes (new, user-visible)

Uh oh!

Caesarsage commented Jul 9, 2026

Uh oh!

Vaishnav88sk commented Jul 22, 2026

Uh oh!

Vaishnav88sk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Vaishnav88sk commented Jul 29, 2026

Uh oh!

Caesarsage commented Jul 29, 2026

Uh oh!

Vaishnav88sk commented Jul 29, 2026

Uh oh!

Caesarsage commented Jul 29, 2026

Uh oh!

Caesarsage commented Jul 29, 2026

Uh oh!

Vaishnav88sk commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Caesarsage commented Jul 30, 2026

Uh oh!

Vaishnav88sk commented Jul 30, 2026

Uh oh!

Harsh4902 commented Jul 30, 2026

Uh oh!

Caesarsage commented Jul 30, 2026

Uh oh!

Harsh4902 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Caesarsage commented Jul 9, 2026 •

edited

Loading

Vaishnav88sk commented Jul 30, 2026 •

edited

Loading